Skip to content

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Jun 27, 2025

Closes WOOMOB-717

⚠️ Please only review this PR if the base PR #15824 has been approved without a need of significant updates

Why

Now that the POS tab is visible for stores in eligible countries but not necessarily eligible for POS, when the merchant taps on the POS tab:

  • There might be async checks for POS eligibility before entering POS, a loading UI is shown while checking POS eligibility: I'm reusing the current loading UI in POS
  • After the eligibility check, if the store is not eligible for POS, an ineligible UI is shown: this requires design, I'm using some basic design first. The ineligible UI should have a CTA to refresh eligibility async, the refresh action will be implemented separately in WOOMOB-720

How

The changes include the addition of a controller to manage eligibility, updates to various views to integrate eligibility checks, and the creation of a new view for handling ineligible states. Below are the most important changes grouped by theme:

POS Eligibility System Implementation

  • POSEntryPointController.swift: Added a new controller to manage POS eligibility, leveraging a feature flag and asynchronous eligibility checks. This controller is observable.
  • POSIneligibleView.swift: Added a new view to display reasons for POS ineligibility and provide users with an option to refresh eligibility. Includes handling for various ineligibility reasons such as unsupported devices, iOS versions, and feature flag status.
  • POSTabEligibilityChecker.swift: Because the site settings async stream's latest value can only be consumed once (the latest value isn't emitted for new subscribers like CurrentValueSubject in Combine), and to avoid duplicate checks in both visibility and eligibility checks, site settings and feature flag are now cached from checkVisibility for usage in checkEligibility when i2 feature is enabled. In a future task WOOMOB-720, the refresh eligibility logic will determine the checks based on the cached eligibility states and the ineligible reason.

Integration of Eligibility Checks into Views

  • PointOfSaleEntryPointView.swift: Updated the POS entry point view to use the new POSEntryPointController. Added logic to display different views based on eligibility state (eligible, ineligible, or loading). [1] [2]
  • POSTabCoordinator.swift: Integrated the eligibility checker into the POS tab coordinator, ensuring it is passed down to the entry point view. [1] [2]

Testing steps

Prerequisite: a store that is in a country eligible for POS, but is missing one condition (e.g. store currency, feature switch for WC version 10.0+, WC version)

  • Launch store in the prerequisite --> the POS tab should appear
  • Tap on the POS tab --> a loading UI that is the same as POS should be shown, and then an ineligible UI should be shown
  • Dismiss the ineligible UI
  • Tap on the POS tab again --> a loading UI -> ineligible UI should be shown

Testing information

  • @jaclync tests stores in the prerequisite, and one that is eligible for POS
  • @jaclync tests when i2 feature flag is off

Example screenshots

Eligible country but ineligible currency:

Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-06-30.at.17.31.53.mp4

Eligible for POS:

Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-06-30.at.17.32.14.mp4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@jaclync jaclync changed the base branch from trunk to feat/WOOMOB-575-separate-pos-tab-visibility June 27, 2025 20:32
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jun 27, 2025

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Number30894
VersionPR #15834
Bundle IDcom.automattic.alpha.woocommerce
Commit33f29a8
Installation URL31ldnigibvl98
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jaclync jaclync changed the title [POS as a tab i2] Add test cases for POSEntryPointController feature flag behavior [POS as a tab i2] Show loading & ineligible UI when tapping on the POS tab Jun 27, 2025
jaclync added 2 commits June 27, 2025 17:09
…eam by caching eligibility results from `checkVisibility`.
@jaclync jaclync added this to the 22.8 milestone Jun 27, 2025
@staskus staskus self-assigned this Jun 30, 2025
PointOfSaleDashboardView()
.environment(posModel)
} else {
PointOfSaleLoadingView()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's this visible shift between one loading state and another. For some reason, even the background color changes.

Simulator.Screen.Recording.-.iPad.Air.13-inch.M2.-.2025-06-30.at.10.56.56.mov

Copy link
Contributor Author

@jaclync jaclync Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this! I was testing in dark mode most of the time and didn't notice it. I can reproduce this especially easily by adding a await Task.sleep(2 * NSEC_PER_SEC) delay to POSTabEligibilityChecker.checkEligibility.

It turns out the background color of the loading state changes because PointOfSaleLoadingView does not have a background color set. Therefore, the background color depends on the parent view's background color. PointOfSaleLoadingView is displayed in PointOfSaleEntryPointView and PointOfSaleDashboardView. Before this PR, the loading state is mostly displayed to the user when embedded in PointOfSaleDashboardView with a background color set while loading the products, the use case in PointOfSaleEntryPointView is just for the POS aggregate model's initialization in task which is almost instant. In this PR, the loading state is being shown from PointOfSaleEntryPointView when checking POS eligibility async but PointOfSaleEntryPointView doesn't have a background color set. In 9feda59, I set the same background color as PointOfSaleDashboardView to PointOfSaleLoadingView so that the loading state has a consistent background color no matter where it is embedded.

before:

Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-06-30.at.13.10.01.mp4

after:

Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-06-30.at.12.43.46.mp4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this!

I wonder if SwiftUI could keep the same animation running while we switch from entry point loading -> products loading. However, let's not address it now; we have this issue with other animations as well. Probably making IndefiniteCircularProgressViewStyle reliant on a current time when showing progress or something like that could make the transitions smooth.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if SwiftUI could keep the same animation running while we switch from entry point loading -> products loading. However, let's not address it now; we have this issue with other animations as well. Probably making IndefiniteCircularProgressViewStyle reliant on a current time when showing progress or something like that could make the transitions smooth.

Yea, certainly worth looking into streamlining the spinner animation for the loading view from 2 different places. I'm also still confirming the loading UI in p1751379425731039-slack-C0354HSNUJH, though I think it's pretty likely we will reuse the spinner animation. Created an issue WOOMOB-741.


private extension POSTabEligibilityChecker {
func checkSiteSettingsEligibility() async -> POSEligibilityState {
if let siteSettingsEligibility {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until the app is killed and relaunched, will we keep the same eligibility state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

POSTabEligibilityChecker's lifetime is per logged-in site session, so it stays the same until switching stores / logout / app relaunched.

In a future task WOOMOB-720, the refresh eligibility logic will refresh site settings with a remote sync when the ineligible reason is related to the site settings. The app also assumes the same store country/currency throughout the site session as the site settings are refreshed only during site initialization. We could also sync the site settings remotely every time tapping on the POS tab, but I felt like that's optimizing the accuracy for a small ratio of edge cases (site settings changed within the site session) with the tradeoff of an additional API request.

Copy link
Contributor

@staskus staskus Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the explanation!

Base automatically changed from feat/WOOMOB-575-separate-pos-tab-visibility to trunk June 30, 2025 15:38
jaclync added 3 commits June 30, 2025 11:45
…change when being shown in different parent views.
…lity / site settings are refreshed where there are multiple subscribers to the same async stream and only the first one gets the value.
@jaclync
Copy link
Contributor Author

jaclync commented Jun 30, 2025

During testing, I noticed an issue when tapping the POS tab (visible from cached value, while checking POS eligibility) before the initial tab visibility check is returned. This is related to the behavior of AsyncStream, where each value can only one consumed by one subscriber (for await), and there's no straightforward way to emit the current value when being subscribed like a current value subject in Combine. I fixed it in 33f29a8 by sharing the async stream subscription task, but I'm not a big fan of this workaround. I will explore other solutions for subscribing to site settings sync outside the POS eligibility checker. Syncing site settings remotely would certainly make implementation easier, but it adds another API request on each POS tab tap.

Copy link
Contributor

@staskus staskus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for addressing the questions

@jaclync jaclync merged commit bc805ac into trunk Jul 1, 2025
13 checks passed
@jaclync jaclync deleted the feat/WOOMOB-717-ineligible-ui branch July 1, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants